Skip to content

Conversation

@zarend
Copy link
Contributor

@zarend zarend commented Dec 28, 2022

Add an opt-out for checkmark indicators for single-selection. Add both and Input and DI token to specify if checkmark indicators are hidden for single-select. By default display checkmark indicators for single-selection. If both DI token and Input are specified, the Input wins.

PR #25890 adds checkmork indicator for single selection. Add an opt-out to provide a way to have same appearance as before #25890.

Does not affect multiple-selection.

Does not affect behavior when avatar is provided. When avatar is provided, display checkmark indicator when selected. This is the same behavior as before #25890.

API Changes

  • Add @Input hideSingleSelectionIndicator to specify if checkmark indicator is displayed for single-selection
  • Add hideSingleSelectionIndicator property to MatChipsDefaultOptions, which specifies default value for hideSingleSelectionIndicator.

@zarend zarend added Accessibility This issue is related to accessibility (a11y) target: minor This PR is targeted for the next minor release area: material/chips labels Dec 28, 2022
Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with a few nits.

@zarend zarend force-pushed the chips-opt-out branch 2 times, most recently from bfd2910 to fe8b637 Compare January 3, 2023 23:20
@zarend
Copy link
Contributor Author

zarend commented Jan 3, 2023

Changes since this was last reviewed

  • moved all the _defaultOptions in this PR so that they're only accessible in the classes that use them.
  • made _hideSingleSelectionIndicator private
  • removed unnecessary assertions in tests

Add an opt-out for checkmark indicators for single-selection. Add both
and Input and DI token to specify if checkmark indicators are hidden for
single-select. By default display checkmark indicators for
single-selection. If both DI token and Input are specified, the Input
wins.

PR angular#25890 adds checkmork indicator for single selection. Add an opt-out
to provide a way to have same appearance as before angular#25890.

Does not affect multiple-selection.

Does not affect behavior when avatar is provided. When avatar is
provided, display checkmark indicator when selected. This is the same
behavior as before angular#25890.

API Changes
 - Add `@Input hideSingleSelectionIndicator` to specify if checkmark
   indicator is displayed for single-selection
 - Add `hideSingleSelectionIndicator` property to
   `MatChipsDefaultOptions`, which specifies default value for
`hideSingleSelectionIndicator`.
@zarend zarend added the action: merge The PR is ready for merge by the caretaker label Jan 4, 2023
zarend added a commit to zarend/components that referenced this pull request Jan 5, 2023
Update the accessibility section on checkmark indicators for
single-selection. Add instructions to always communicate selection with
icon indicators. Fulfill documentation needs as follow-up for angular#25890 and
 angular#26338.
@zarend zarend merged commit 5e96eb0 into angular:main Jan 6, 2023
zarend added a commit to zarend/components that referenced this pull request Jan 9, 2023
Update the accessibility section on checkmark indicators for
single-selection. Add instructions to always communicate selection with
icon indicators. Fulfill documentation needs as follow-up for angular#25890 and
 angular#26338.
zarend added a commit to zarend/components that referenced this pull request Jan 9, 2023
Update the accessibility section on checkmark indicators for
single-selection. Add instructions to always communicate selection with
icon indicators. Fulfill documentation needs as follow-up for angular#25890 and
 angular#26338.
zarend added a commit to zarend/components that referenced this pull request Jan 9, 2023
Update the accessibility section on checkmark indicators for
single-selection. Add instructions to always communicate selection with
icon indicators. Fulfill documentation needs as follow-up for angular#25890 and
 angular#26338.
zarend added a commit to zarend/components that referenced this pull request Jan 10, 2023
Update the accessibility section on checkmark indicators for
single-selection. Add instructions to always communicate selection with
icon indicators. Fulfill documentation needs as follow-up for angular#25890 and
 angular#26338.
zarend added a commit to zarend/components that referenced this pull request Jan 10, 2023
Update the accessibility section on checkmark indicators for
single-selection. Add instructions to always communicate selection with
icon indicators. Fulfill documentation needs as follow-up for angular#25890 and
 angular#26338.
zarend added a commit to zarend/components that referenced this pull request Jan 10, 2023
Update the accessibility section on checkmark indicators for
single-selection. Add instructions to always communicate selection with
icon indicators. Fulfill documentation needs as follow-up for angular#25890 and
 angular#26338.
angular-robot bot pushed a commit that referenced this pull request Jan 10, 2023
…#26375)

Update the accessibility section on checkmark indicators for
single-selection. Add instructions to always communicate selection with
icon indicators. Fulfill documentation needs as follow-up for #25890 and
 #26338.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Accessibility This issue is related to accessibility (a11y) action: merge The PR is ready for merge by the caretaker area: material/chips target: minor This PR is targeted for the next minor release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants